Skip to content

Added enum types and verification#168

Open
cheestree wants to merge 13 commits intoliquid-java:mainfrom
cheestree:main
Open

Added enum types and verification#168
cheestree wants to merge 13 commits intoliquid-java:mainfrom
cheestree:main

Conversation

@cheestree
Copy link

@cheestree cheestree commented Mar 3, 2026

Closes #23. This pull request introduces support for Java enums in the refinement type system and SMT translation. The changes include updates to the grammar, AST, visitors, and Z3 translation logic, as well as new test cases for correct and incorrect enum usages.

image

An enumerate rule and ENUM token for expressions like Color.Red and the respective AST node to represent enum literals.
The ExpressionVisitor was updated to handle and map enum literals with Z3's enum sorts and constants, while the translator creates and caches these.
The MethodsFirstChecker and RefinementTypeChecker have also been modified to add into the global context instead of just local and to recognize field accesses and annotate them with the proper refinement metadata for type checking

A set of tests was added to verify the valid/invalid behaviour of enum refinements, both Refinement and StateRefinement.

Added a test for enums to, in the future, help clean the refinement message.
Copy link
Collaborator

@rcosta358 rcosta358 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Left some feedback about the parsing, representation, scoping and translation of enums.
Additionally, we need more tests to verify the correctness of this implementation.

ID_UPPER '(' args? ')';

enumCall:
OBJECT_TYPE;
Copy link
Collaborator

@rcosta358 rcosta358 Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand you used OBJECT_TYPE because of the rule ambiguity. However, this raises a concern, because it allows for multiple chaining of ids (e.g., Mode.photo.photo, Mode.photo.photo.photo, etc.) which should not be syntactically valid.

We could avoid the ambiguity by forcing the OBJECT_TYPE to always start with a lowercase letter and declare enumCall as ID '.' ID, but Java packages are allowed start with an uppercase letter (even though it's highly discouraged).

So, we can introduce another terminal, just for enums:

enumCall: 
	ENUM_CALL;

// ...

ENUM_CALL: [a-zA-Z_][a-zA-Z0-9_]* '.' [a-zA-Z_][a-zA-Z0-9_]*;

This way, you guarantee the enums are always represented by two ids separated by a dot, without ambiguity.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's understandable, my issue with that, at the time of development, was if ID and ID_UPPER wouldn't, even if they are slightly different, lead to ambiguity due to other grammar rules. I should've explored more formats.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have support for enums that live in another package, via qualified name. I agree with @rcosta358 that we want a first version working, and then we can think about this. To achieve this, we need a name resolution (à lá Spoon).

Comment on lines +240 to +246
private String enumCreate(EnumContext rc) {
String enumText = rc.enumCall().getText();
int lastDot = enumText.lastIndexOf('.');
String enumClass = enumText.substring(0, lastDot);
String enumConst = enumText.substring(lastDot + 1);
String varName = String.format(Formats.ENUM_VALUE, enumClass, enumConst);
return varName;
Copy link
Collaborator

@rcosta358 rcosta358 Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the "enum#%s#%s" format because it's hard to read in the error messages. Here you can simply return the enumText, and everywhere else still use this format but change it to "%s.%s" instead.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should've gone with a better format, I admit. I wasn't sure due to the rest of the existing types.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to "decode" enums when they come back in the error messages back to their Refinement version. Two approaches: 1) having a translation map. 2) storing the source of each variable in the Verification Condition (I'm doing this in aeon, and I think we discussed it for LJ).

This is not specific for Enums, we might want to do this for other types.

String varName = String.format(Formats.ENUM_VALUE, enumName, ev.getSimpleName());
Predicate refinement = Predicate.createEquals(Predicate.createVar(varName),
Predicate.createLit(String.valueOf(ordinal), Types.INT));
context.addGlobalVariableToContext(varName, qualifiedEnumName, enumRead.getReference(), refinement);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why global? Shouldn't it only be visible within the class or package where it was declared?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is an enum that is used across methods, and a shared package is needed, when the context is reinitialized, the local context is cleaned. If it's kept in the global context, the placement is irrelevant.
For example, the following common package a layer above the testSuite.

image

When line 134 is changed to context.addVarToContext(varName, enumRead.getReference(), refinement, enumRead), the following occurs:

image

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not really a shared package because it's not even included in the verification, each test is ran individually from its file or folder.

Copy link
Collaborator

@rcosta358 rcosta358 Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is: if I declare an enum X in class A, should I be able to use it in class B? It might be fine, just raising some questions about how it should work.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, if we can use enums from different classes, we should make sure they have their fully qualified name to distinguish enums with the same name from different classes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realize the testing was that restrictive. I can try and create some test scenarios that take into consideration global and local context. The other option would be to restrict enum verification to only in the same file, but that feels like a cop-out instead of an actual fix/intended functionality. 🤔

@rcosta358 rcosta358 added the enhancement New feature or request label Mar 4, 2026
Created AST node and visitors for enums.
Changed grammar as suggested to specific rules and positioning to avoid ambiguity.
Changed format.
@cheestree
Copy link
Author

image

The change to using EnumSort seems to have worked, but I'm still unsure of how exactly tests for this should be created if some problematic areas are based off of package/class location. Is there a currently implemented test that runs off of its own folder and has multiple components in different files?

@rcosta358
Copy link
Collaborator

Currently every test file/folder is verified completely isolated from one another. Everything needed for it should be in the corresponding file/folder, even if that means unwanted repetition.

Fixed grammar so that it required uppercase start for both parts. Differentiates between types and calls.
This reverts commit c831921.
This reverts commit d785934.
Translation is now done in 2 phases: a first one for enums, and storing their constant values and names, and the second one as previously done, but with special attention now due to the presence of enums.
If the variable, when translating, is already present, it isn't added, and there are no duplicates.
@cheestree
Copy link
Author

image

This seems to be the best outcome for now, as enums aren't duplicated if present via checking using a map. It looks like a O(n^2) situation but since there aren't usually a lot of enum values for a single type, it is (in my opinion) negligible.

Added 2 error tests for enum: when setting null on a field with an enum refinement, and on a function parameter refinement. A successful error test was added as well to test backtracking to a null state.
Copy link
Collaborator

@rcosta358 rcosta358 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some minor comments.

Added some more enum tests for different situations (like as parameters or fields with enum refinements).
Merged CorrectEnumResetMode into CorrectEnumUsage (redundant having a separate one that only tests resetting to null)
Refactored enumCreate as suggested.
Refactored grammar to follow Enumerate class.
Moved and refactored the RefinmentMessage test to be a very simple Refinement checker.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Enums to refinement language and verification as parameters.

3 participants